Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add warning message about release version to dnf plugin #3385

Merged
merged 2 commits into from
Mar 28, 2024

Conversation

jirihnidek
Copy link
Contributor

@jirihnidek jirihnidek commented Mar 12, 2024

  • Card ID: CCT-171
  • New feature: when a release is set by subscription-manager, then dnf plugin "subscription-manager" prints warning about this release
  • The information about release is read only from cache file to not slow down DNF.

@cnsnyder cnsnyder requested review from a team and ptoscano and removed request for a team March 12, 2024 15:41
@jirihnidek jirihnidek force-pushed the jhnidek/dnf_plugin_releasever branch from 60eab4a to aba1178 Compare March 12, 2024 16:14
Copy link

github-actions bot commented Mar 12, 2024

Coverage

Coverage (computed on Fedora latest) •
FileStmtsMissCoverMissing
subscription_manager
   cache.py63111881%30–34, 75, 82, 90, 96, 101–103, 124, 130–132, 145–148, 151, 199, 201, 240–243, 250–253, 270, 273–275, 293, 302–303, 305, 314, 354–355, 389–390, 421, 423–424, 426, 439, 488, 492, 497, 502–505, 508, 516, 519–520, 541, 593, 596, 621, 726, 744, 779, 781–782, 811, 814–820, 823, 855–858, 860, 871, 889, 911–912, 952–955, 957, 985, 1014–1015, 1042–1043, 1066–1067, 1071, 1075, 1077, 1085, 1127, 1155–1161, 1163, 1165, 1172–1175, 1177
TOTAL18391469774% 

Tests Skipped Failures Errors Time
2647 14 💤 0 ❌ 0 🔥 40.576s ⏱️

@jirihnidek jirihnidek force-pushed the jhnidek/dnf_plugin_releasever branch 2 times, most recently from 094f183 to d0d0370 Compare March 13, 2024 14:25
@jirihnidek jirihnidek marked this pull request as ready for review March 13, 2024 15:19
@cnsnyder cnsnyder requested a review from a team March 13, 2024 15:20
Copy link
Contributor

@ptoscano ptoscano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to work most of the time, there is a bug I found out while testing (explained later).


The first commit factors out code from ReleaseCommand._get_consumer_release() into a new get_consumer_release() helper, using it also in the dnf plugin; then the second commit stops using get_consumer_release() in the dnf plugin, leaving this helper used only by its original user (ReleaseCommand._get_consumer_release()). Since these changes are not actually needed for the changes in the dnf plugin, then I'd suggest to simplify the changes in this PR:

  • squash the first and the second commit together, since it is enough one commit to change the dnf plugin
  • drop the changes in src/subscription_manager/cli_command/release.py and src/subscription_manager/release.py altogether

I tried to first --set a release, then --unset it; the releaseVer attribute of the consumer is now an empty string:

# cat /var/lib/rhsm/cache/releasever.json ; echo
{"releaseVer": ""}

this results in the following output:

# dnf update
Updating Subscription Management repositories.

This system has release set to  and it receives updates only for this release.

Last metadata expiration check: 0:09:54 ago on Wed Mar 27 06:36:24 2024.
[...]

ReleaseCommand.show_current_release() handles both None and empty:

release = self._get_consumer_release()
if release:

So most likely the dnf plugin ought to do that too; i.e.

                    if release_version is not None:

into

                    if release_version:

Thanks!

src/plugins/dnf/subscription_manager.py Outdated Show resolved Hide resolved
src/plugins/dnf/subscription_manager.py Outdated Show resolved Hide resolved
@jirihnidek jirihnidek force-pushed the jhnidek/dnf_plugin_releasever branch 2 times, most recently from d1e7016 to 707f9b4 Compare March 27, 2024 13:26
* Card ID: CCT-171
* New feature: when a release is set by subscription-manager,
  then dnf plugin "subscription-manager" prints warning about
  this release
* Information about release is read only from cache file.
  When there is cache file containing information about
  current release, then only this information is used
  for printing warning about release lock. Gathering such
  information from candlepin would unnecessarily slow
  down DNF.
@ptoscano ptoscano force-pushed the jhnidek/dnf_plugin_releasever branch from 707f9b4 to 9979053 Compare March 28, 2024 11:08
Copy link
Contributor

@ptoscano ptoscano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@ptoscano ptoscano merged commit a821f9c into main Mar 28, 2024
17 checks passed
@ptoscano ptoscano deleted the jhnidek/dnf_plugin_releasever branch March 28, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants